Skip to content

fix: Rename builtin discussion provider, "edX" -> "Open edX"#2660

Merged
kdmccormick merged 4 commits intomasterfrom
kdmccormick/openedx-discussion-provider
Nov 18, 2025
Merged

fix: Rename builtin discussion provider, "edX" -> "Open edX"#2660
kdmccormick merged 4 commits intomasterfrom
kdmccormick/openedx-discussion-provider

Conversation

@kdmccormick
Copy link
Copy Markdown
Member

@kdmccormick kdmccormick commented Nov 17, 2025

Description

It seems wrong that the default providers are called "edX" and "edX (New)".

This PR changes the UI-level provider names to "Open edX" and "Open edX (New)".

Perhaps other names would be more appropriate. I'm open to any input from Product/UX here. I'm also not certain whether it's correct to have the openedx provider called "(New)", rather than calling the legacy provider "(Old)", but I'll leave that up to anyone with more context than I on the migration.

[update based on Aamir's feedback] This PR changes the UI-level provider names to "Open edX (legacy)" and "Open edX".

The internal provider codenames, legacy and openedx, are unchanged.

Supporting information

Before

Screenshot 2025-11-17 at 2 40 59 PM

After

Screenshot 2025-11-17 at 2 42 13 PM

Testing instructions

In tutor dev, this screen is visible at http://apps.local.openedx.io:2001/authoring/course/{courseId}/pages-and-resources/discussion/settings

Merge constraints

No particular urgency, but I'll backport this to Ulmo if it lands in time.

Best Practices Checklist

We're trying to move away from some deprecated patterns in this codebase. Please
check if your PR meets these recommendations before asking for a review:

  • Any new files are using TypeScript (.ts, .tsx).
  • Avoid propTypes and defaultProps in any new or modified code.
  • Tests should use the helpers in src/testUtils.tsx (specifically initializeMocks)
  • Do not add new fields to the Redux state/store. Use React Context to share state among multiple components.
  • Use React Query to load data from REST APIs. See any apiHooks.ts in this repo for examples.
  • All new i18n messages in messages.ts files have a description for translators to use.
  • Avoid using ../ in import paths. To import from parent folders, use @src, e.g. import { initializeMocks } from '@src/testUtils'; instead of from '../../../../testUtils'

@kdmccormick
Copy link
Copy Markdown
Member Author

kdmccormick commented Nov 17, 2025

@ormsbee , with your work on the discussions migration, do you have a sense of whether this rename is correct and/or know someone who might?

@marcotuts
Copy link
Copy Markdown

Open EdX an Open EdX (legacy) feels more appropriate in my mind, plus one to this change overall as well.

I don't know if the old UI is marked deprecated but it for sure should be legacy and even maybe not default configured. That's probably a separate DEPR conversation

@ayub02
Copy link
Copy Markdown

ayub02 commented Nov 18, 2025

@kdmccormick i chose this naming convention back when my team at 2U was rolling out the discussions MFE and discussion sidebar (2022-23).

I agree with you that:

  1. edX should be replaced with Open edX
  2. Provider names should be: Open edX (legacy) and Open edX.

Comment thread src/pages-and-resources/discussions/app-config-form/messages.js Outdated
Comment thread src/pages-and-resources/discussions/app-config-form/messages.js Outdated
@kdmccormick
Copy link
Copy Markdown
Member Author

Thanks @ayub02 , done.

@kdmccormick kdmccormick enabled auto-merge (squash) November 18, 2025 14:12
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.83%. Comparing base (daed664) to head (981057b).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2660   +/-   ##
=======================================
  Coverage   94.83%   94.83%           
=======================================
  Files        1234     1234           
  Lines       27764    27764           
  Branches     6263     6283   +20     
=======================================
  Hits        26331    26331           
  Misses       1362     1362           
  Partials       71       71           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kdmccormick kdmccormick merged commit 5fadcca into master Nov 18, 2025
7 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/openedx-discussion-provider branch November 18, 2025 14:55
@kdmccormick
Copy link
Copy Markdown
Member Author

kdmccormick commented Nov 18, 2025

I don't know if the old UI is marked deprecated but it for sure should be legacy and even maybe not default configured. That's probably a separate DEPR conversation

agreed. I don't have the bandwidth to take on that level of change right now, but if anyone involved with the Discussions is reading this, please feel empowered to propose these changes!

@ormsbee
Copy link
Copy Markdown
Contributor

ormsbee commented Nov 18, 2025

One bit of nuance on this is that the alternatives are third-party services and authors may not understand that they're running "Open edX" at all--they just know it by whatever the branding is on that particular instance. The choice that an author is making is, "Am I running the forums that are built into the LMS or am I using one of these third party services that I might already be familiar with?" From that perspective, I can see a case for using the site name.

To be clear, I still think "edX" is the wrong thing here for the community as a whole. I think we should consider re-framing it more explicitly as "on-platform/built-in" vs. "third-party" forum services, though I'm not sure what the exact wording should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants